Skip to content

Expose Failed Patient Transfers via Status#1486

Open
trobanga wants to merge 1 commit intomainfrom
1485-cda-endpoint-failed-patient-transfers
Open

Expose Failed Patient Transfers via Status#1486
trobanga wants to merge 1 commit intomainfrom
1485-cda-endpoint-failed-patient-transfers

Conversation

@trobanga
Copy link
Contributor

@trobanga trobanga commented Feb 25, 2026

Summary

  • Adds a new endpoint GET /api/v2/process/status/{processId}/failed_patients returning patient IDs and error messages for failed transfers
  • Records per-patient errors in TransferProcessStatus via a failedPatients list (excluded from /status JSON via @JsonIgnore to keep status responses concise)
  • Separates incSkippedBundles() from addFailedPatient() for independent tracking of skipped bundles and error details
  • Fixes errorOnSecond test helpers to correctly error on exactly the 2nd call using AtomicInteger instead of AtomicBoolean

Closes #1485

@trobanga trobanga force-pushed the 1485-cda-endpoint-failed-patient-transfers branch 5 times, most recently from 185502b to c1915cc Compare February 25, 2026 12:46

private static Deidentificator errorOnSecond(TransportBundle bundle) {
var first = new AtomicBoolean(true);
var callCount = new AtomicInteger(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert that

@trobanga trobanga force-pushed the 1485-cda-endpoint-failed-patient-transfers branch 2 times, most recently from c829665 to ca715ea Compare February 26, 2026 14:43
@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.34%. Comparing base (5ed277e) to head (55de5b2).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #1486   +/-   ##
=========================================
  Coverage     99.34%   99.34%           
- Complexity      555      558    +3     
=========================================
  Files           127      127           
  Lines          1975     1996   +21     
  Branches         62       62           
=========================================
+ Hits           1962     1983   +21     
  Misses            2        2           
  Partials         11       11           
Files with missing lines Coverage Δ
...re/smith/fts/cda/DefaultTransferProcessRunner.java 100.00% <100.00%> (ø)
...java/care/smith/fts/cda/TransferProcessStatus.java 100.00% <100.00%> (ø)
.../smith/fts/cda/rest/TransferProcessController.java 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@trobanga trobanga force-pushed the 1485-cda-endpoint-failed-patient-transfers branch 4 times, most recently from eb98b03 to 01566b3 Compare February 27, 2026 08:42
Copy link
Member

@alexanderkiel alexanderkiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also miss documentation about the new endpoint. Or is than generated via OpenAPI?


private <T> Mono<T> handlePatientError(
String step, String patientId, Step stepEnum, Throwable e) {
logError(step, patientId, e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The step string should come from the step enum.

.flatMap(this::sendBundleForPatient, config.maxSendConcurrency)
.doOnNext(b -> status.updateAndGet(TransferProcessStatus::incSentBundles))
.onErrorContinue((e, r) -> status.updateAndGet(TransferProcessStatus::incSkippedBundles));
.doOnNext(b -> status.updateAndGet(TransferProcessStatus::incSentBundles));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the onErrorContinue removed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onErrorContinue was replaced by onErrorResume(... -> Mono.empty()) inside each per-patient method (selectDataForPatient, deidentifyForPatient, sendBundleForPatient). This converts errors to empty publishers before they reach the outer flux, making onErrorContinue at the flux level unnecessary.

This approach is also safer — onErrorContinue has well-known pitfalls where it doesn't work as expected with certain operators (e.g. flatMap), because it relies on operator-level support. onErrorResume is the more predictable pattern.

@trobanga trobanga force-pushed the 1485-cda-endpoint-failed-patient-transfers branch from 01566b3 to 33651d5 Compare February 27, 2026 10:39
@trobanga trobanga force-pushed the 1485-cda-endpoint-failed-patient-transfers branch from 33651d5 to 55de5b2 Compare February 27, 2026 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add or Extend CDA Enpoint to Return IDs and Errors of Failed Patient Transfers

2 participants